Add Conditional Randomization Test #359
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #359 +/- ##
==========================================
+ Coverage 98.10% 99.73% +1.62%
==========================================
Files 22 20 -2
Lines 1161 1120 -41
==========================================
- Hits 1139 1117 -22
+ Misses 22 3 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bthirion
left a comment
bthirion
left a comment
There was a problem hiding this comment.
I'm confused by the fact that this includes a big diff.
I made some comments specific to the CRT part.
| self.n_repeat, -1 | ||
| ) | ||
|
|
||
| self.importances_ = np.mean(np.abs(self.test_scores_), axis=0) |
There was a problem hiding this comment.
I don't understand whuy you take an absolute value here.
There was a problem hiding this comment.
I take the absolute value because there is no guarantee that the test_score is always positive or negative.
In consequence, taking the mean can give a value close to zeros just because there is a switch between positive and negative values, which is, for me, not a desirable behaviour.
There was a problem hiding this comment.
I would take the absolute value after the mean, not before. I do agree that the importance measure should only take into account the fact that there is an effect, not that it is negative or positive. However, by taking the absolute value before the mean you are not reducing the bias. If you take the absolute value after the mean, as the importance should be centered around 0 as it is just noise, we are going to reduce the variance and achieve better importance discrimination.
|
The sampler of the CRT is simpler than the one of the Knockoffs. For the Knockoffs there is need to have sequential sampler (in practice, for the Gaussian sampler, optimize the parameter s). For the CRT (similarly as with the CFI), it can be done in parallel. |
3 participants
This PR is based on
